Suggestion: Move parameter into module in number_test#1303
Suggestion: Move parameter into module in number_test#1303Keno wants to merge 1 commit intochipsalliance:masterfrom
Conversation
I'm working on validating a parser for Verilog-AMS, which is basically vanilla verilog + extensions + some backports from SystemVerilog. Because of this shared heritage, quite a number of the tests in this repository are applicable and helpful. The number_test tests are useful also, because they test lexing of numberic literals. There are two tests (_3 and _4) that test SystemVerilog extensions, but the remainder are Vanilla verilog modulo the fact that `parameter` statements can't appear at top level in vanilla Verilog. By moving the parameter statements inside the model, the test would be valid in both vanilla and System Verilog. I understand that this is not a Verilog-AMS test suite, but I was hoping you might be open to generalizing the test files where the particular feature being tested itself is not a SystemVerilog extension, such that we may share some of these testcases between this repository and our Verilog-AMS test suite. Signed-off-by: Keno Fischer <keno@juliacomputing.com>
|
I personally have no objection, but we do want to have some tests that exercise the SystemVerilog functionality so I do not agree with just switching all these to be declared inside the module. I'll also comment ivtest has some Verilog-A tests, but they are obviously not loaded by sv-tests. |
|
Given that these tests strictly just test for parseability of number literals, I am not opposed to making these also digestable by pure Verilog parsers. One request though: please make sure to indent the parameter line within the module block. Not sure if there is even a super-consistent style (2 or 4 spaces) in the test-suite, but I suggest 2. module test;
parameter test = 0;
endmodule@caryr which is the test you'd be worried about ? Or in general you suggest to have at least one test that has a parameter outside the module body ? For that, I'd probably suggest that we add a test in |
hzeller
left a comment
hzeller
left a comment
There was a problem hiding this comment.
LGTM, but please add the proper indentation.
Once we know how to proceed with 'parameter outside module' question, I might also ask you to add one test of a parameter outside a module in the chapter-6 area to cover the accidental covering of that feature.
|
My request is to have at least one with the parameter outside the module definition to check that parameters can be declared outside a module. To be thorough we would want to cover every combination both inside and outside the module, but that is likely being overly cautious. I'm fine with moving all these inside for this section and creating a copy to check that out of module parameter declarations work correctly in a different section. |
3 participants
I'm working on validating a parser for Verilog-AMS, which is
basically vanilla verilog + extensions + some backports from
SystemVerilog. Because of this shared heritage, quite a number
of the tests in this repository are applicable and helpful.
The number_test tests are useful also, because they test
lexing of numeric literals. There are two tests (_3 and _4)
that test SystemVerilog extensions, but the remainder are
Vanilla verilog modulo the fact that
parameterstatementscan't appear at top level in vanilla Verilog. By moving
the parameter statements inside the model, the test would
be valid in both vanilla and System Verilog. I understand
that this is not a Verilog-AMS test suite, but I was hoping
you might be open to generalizing the test files where the
particular feature being tested itself is not a SystemVerilog
extension, such that we may share some of these testcases
between this repository and our Verilog-AMS test suite.